Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make events not require sign-in #228

Merged
merged 14 commits into from
Apr 24, 2024
Merged

Make events not require sign-in #228

merged 14 commits into from
Apr 24, 2024

Conversation

SheepTester
Copy link
Member

@SheepTester SheepTester commented Apr 14, 2024

Info

there's no issue for this

event pages now no longer require login (because the API never required it). this way, you can send links to the events in Discord and it'll show the event cover bigly

however, to encourage people to sign up, these pages have a massive banner telling people to log in. the login form is embedded so you can just click log in if you have a password saved

there are otherwise no changes to the user's experience when signed in

Changes

  • event page doesn't require log in
    • also added preview images for several pages, and the event page's preview image is LARGE on discord
  • events page doesn't require login
  • login appeal component that includes the login form that shows when the user is logged out
    • useful if/when store becomes readonly for non-signed in users too
  • i also refactored all the pages so that withAccessType passes it the auth token (since it already reads it from the cookie)

Type of Change

  • Bug Fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as
    expected)
  • Logistics Change (A change to a README, description, or dev workflow setup like
    linting/formatting)
  • Continuous Integration Change (Related to deployment steps or continuous integration
    workflows)
  • Other: (Fill In)

Testing

I have tested that my changes fully resolve the linked issue ...

  • locally on Desktop.
  • on the live deployment preview on Desktop.
  • on the live deployment preview on Mobile.
  • I have added new Cypress tests that are passing.

Checklist

  • I have performed a self-review of my own code.
  • I have followed the style guidelines of this project.
  • I have documented any new functions in /src/lib/* and commented hard to understand areas
    anywhere else.
  • My changes produce no new warnings.

Screenshots

image

image

image

image

i also fixed the blue (for some reason Discord parses #fff as #00f, oof)

image

Copy link

vercel bot commented Apr 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
membership-portal-ui-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 23, 2024 11:55pm
testing-membership-portal-ui-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 23, 2024 11:55pm

@farisashai
Copy link
Member

image

I think this might be too big on mobile since there's essentially no relevant content to the events page above the fold. Maybe expandable login section with just an alert showing or smth smaller in general

@farisashai
Copy link
Member

there's no way to get to the single
event page from the events page modals to share a link to them ?

@farisashai
Copy link
Member

If you click sign up from the events page login popup and then go back, it takes you from the /register page to the login page instead of the page you were previously on. I think this might have been hardcoded in initially

@farisashai
Copy link
Member

You should hide the attended/not attended filter if the events page isn't logged in

@SheepTester
Copy link
Member Author

If you click sign up from the events page login popup and then go back, it takes you from the /register page to the login page instead of the page you were previously on. I think this might have been hardcoded in initially

I'm not able to reproduce this for individual event pages

  1. Go to event page, e.g. https://membership-portal-ui-v2-git-sean-public-event-acmucsd.vercel.app/events/e154acf7-3465-49e4-baa6-f40bfa2f4ad8
  2. Go back
  3. It takes me back to the event page

For /events, going to "Sign up" and back leaves me on the /register page but changes the URL to /events. I think this is due to a bug in the event's page's filtering implementation rather than the registration page

there's no way to get to the single event page from the events page modals to share a link to them ?

I would like to add "Events" to the navbar even when signed out. I think I'll do this in a separate PR after the store page also becomes public

I think the current navbar implementation isn't great (has duplicate elements for mobile and desktop; has a hardcoded height; uses signed-out navbar on 404 pages) so I'd like to refactor it in a separate PR, along with adding events and store for the logged out navbar

image

I think this might be too big on mobile since there's essentially no relevant content to the events page above the fold. Maybe expandable login section with just an alert showing or smth smaller in general

I think the size is fine. We'd rather people stay logged in so we should try to pester them to log in, and it's easy to scroll past anyways

Copy link
Contributor

@alexzhang1618 alexzhang1618 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving so this isn't blocked, but some questions

src/components/common/Login/index.tsx Outdated Show resolved Hide resolved
src/lib/api/EventAPI.ts Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants